We enabled the build-time switch for dev channels in bug 1097128, and that appears to have gone smoothly. So we should take the next step of removing the compile-time conditional altogether, to clean up the code and provide a uniform basis across all channels for things like heycam's logical-properties style system work. Actually exposing support for vertical mode remains dependent on the runtime pref (bug 1099032), which we'll switch when it is considered ready to ship.

Comment on attachment 8546568 [details] [diff] [review] remove the compile-time switch to enable/disable vertical writing mode support Review of attachment 8546568 [details] [diff] [review]: ----------------------------------------------------------------- Definitely

Is it worth running some performance tests comparing: - compile time switch off, and - compile time and runtime switches on before removing the compile time switch?

(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #3) > Is it worth running some performance tests comparing: > - compile time switch off, and > - compile time and runtime switches on > before removing the compile time switch? I did some comparisons before we toggled the compile-time switch for dev builds, but OK, let's try again with current code. I pushed tryserver PGO talos jobs to compare current trunk code (where the compile-time switch is on, so vertical support is present but preffed-off), a job with the compile-time switch turned off (so we'd expect this to perform better than trunk), and one with the runtime pref turned on (which I would expect to make essentially no difference as I presume none of the talos tests actually use writing-mode and friends). Resulting figures for the Tp test (in each case, this is the mean of 5 runs, along with their sample standard deviations, and the percentage variation from current trunk): Current Compile-OFF Runtime-ON Linux 251.856 249.354 248.89 s=2.33 s=3.33 s=3.54 -0.99% -1.18% Linux64 221.636 221.264 216.42 s=1.06 s=1.59 s=1.00 -0.17% -2.35% WinXP 187.522 186.144 191.626 s=1.40 s=1.36 s=0.73 -0.73% +2.19% Win7 205.946 202.682 206.236 s=0.53 s=0.90 s=1.55 -1.58% +0.14% Win8 190.350 191.33 191.534 s=0.60 s=1.51 s=1.43 +0.51% +0.62% It looks like in general, disabling vertical support altogether at compile time may give us a marginal gain - which is what we'd expect - although only the Win7 figure looks statistically significant based on this small sample. It's also interesting to note that enabling the runtime writing-mode pref apparently gives us an equally-significant *win* on Linux to the possible regression on Windows. I don't believe that can actually be true, given that (a) there's nothing platform-specific about the code in question here, and (b) the runtime pref should not affect Tp pageload as I highly doubt the CSS feature is used there at all; so I think we're just seeing noise here, and this sample isn't big enough for it to average out. I'm not seeing anything that makes me worry unduly about the performance implications, anyhow.